fix(firestore): generate unique listener IDs on web#989
Open
robingenz wants to merge 2 commits into
Open
Conversation
Listener IDs were derived from `Date.now()`, so listeners created in the same millisecond shared an ID and overwrote each other in the unsubscribes map, leaving the earlier ones unable to be removed. Replace the timestamp with a per-instance incrementing counter. The same issue was present in the storage plugin (downloadFile/uploadFile callback IDs) and is fixed here as well. Closes #986
robingenz
added
bug/fix
There was a problem hiding this comment.
Pull request overview
This PR fixes a web-side listener/callback ID collision issue by replacing Date.now().toString() with per-instance incrementing counters, preventing overwrites when multiple listeners/callbacks are created within the same millisecond (addressing #986).
Changes:
- Firestore (web): generate unique snapshot listener IDs via an incrementing counter and store them in
unsubscribesMap. - Storage (web): generate unique callback IDs for
downloadFile/uploadFilevia an incrementing counter. - Add a changeset bumping both
@capacitor-firebase/firestoreand@capacitor-firebase/storageas patch releases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/storage/src/web.ts | Replaces time-based callback IDs with per-instance incrementing IDs for web download/upload callbacks. |
| packages/firestore/src/web.ts | Replaces time-based listener IDs with per-instance incrementing IDs to avoid unsubscribesMap collisions. |
| .changeset/firestore-storage-unique-listener-ids.md | Adds a patch changeset documenting the fix for both packages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
@capacitor-firebase/analytics
@capacitor-firebase/app
@capacitor-firebase/app-check
@capacitor-firebase/authentication
@capacitor-firebase/crashlytics
@capacitor-firebase/firestore
@capacitor-firebase/functions
@capacitor-firebase/messaging
@capacitor-firebase/performance
@capacitor-firebase/remote-config
@capacitor-firebase/storage
commit: |
17 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Date.now().toString()listener ID generation inFirebaseFirestoreWebwith a per-instance incrementing counter, so multiple snapshot listeners registered in the same millisecond no longer collide and overwrite each other inunsubscribesMap(the earlier listeners then couldn't be removed).FirebaseStorageWeb, wheredownloadFileanduploadFilereturned non-unique callback IDs for the same reason.Test plan
removeSnapshotListener.npm run buildfor both packages succeeds.Closes #986